Add type ChordalGMRF.#81
Merged
Merged
Conversation
Extract solver abstraction helpers (_ga_init_solver, _ga_update_and_solve!, _ga_make_posterior) so the Newton iteration and IFT-based rrule each exist once, dispatching on GMRF type. Removes ~130 lines of duplicated code.
Add 4-arg SparseMatrixCSC constructor that wraps Q in Hermitian, matching
the 2-arg constructor's behavior. Fix rrule dispatch from ::typeof(ChordalGMRF)
(resolves to ::UnionAll, matching GMRF too) to ::Type{ChordalGMRF}.
The rrules for HermSparse/SymSparse cause method invalidation that exposes
an upstream ChainRulesCore bug: ProjectTo{SymTridiagonal} extracts only one
triangle of the off-diagonal, losing the factor of 2 from symmetry.
Contributor
Author
|
I am going to rewrite this all using Mooncake; probably today. |
Owner
|
Ah, alright! I didn't make any major changes to it, just restructured things a bit to avoid code duplication particularly for |
Contributor
Author
|
@timweiland It is ready. The |
Owner
|
I noticed MooncakeSparse.jl is registered now, nice!
Fresh install fails to resolve. DI maintainers are aware though and this should be resolved quite soon from what I've seen on the Julia Slack, so I'm fine with simply waiting for that to happen |
Contributor
Author
|
Annoying. What if you relax the compat in your your local MooncakeSparse.jl to |
Owner
|
As far as I can tell MooncakeSparse.jl uses the new API that was only introduced in 0.5.25, in particular the whole friendly tangent business |
4 tasks
- Replace the broken vendored deps/MooncakeSparse submodule with the registered MooncakeSparse 0.1 (Mooncake 0.5.25 compat now satisfiable by DifferentiationInterface 0.7.17). - Drop unused `chordal` and `triangular` imports from CliqueTrees.Multifrontal — `chordal` was removed in CliqueTrees 1.19.2 and neither symbol was referenced. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lost in 8e31503 (Zygote -> Mooncake) when src/piracy.jl was removed. Still needed for the Zygote-tagged GMRF constructor tests: ChainRulesCore's ProjectTo{SymTridiagonal} drops the factor of 2 from symmetry on the off-diagonal, and the explicit sum rrule sidesteps it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a docstring covering the role (Mooncake-friendly chordal Cholesky backing), fields, and construction modes. Include the type in docs/src/reference/gmrfs.md alongside GMRF. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The test asserts the NB→Poisson Hessian convergence at rtol=1e-6 with η = randn(5). Unlucky draws push exp(η) large enough that the 1/r correction at r=1e8 exceeds tolerance. Pin the seed so the assertion is reproducible. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two issues prevented the chordal autodiff tests from running cleanly once the branch was rebased onto the workspace changes on main: 1. The logpdf rrule was registered for `::AbstractGMRF`, which catches `ChordalGMRF` and then crashes accessing `x.linsolve_cache.alg` inside the fallback error message. Restrict to `::GMRF` so ChordalGMRF falls through to native Mooncake AD (which already gives correct gradients via MooncakeSparse's mul!/dot rrules). 2. The chordal test file shared two top-level names with the non-chordal one: `backends` and `test_gauss_approx_pipeline`. Both files include into the same module, so the chordal redefinitions shadowed the non-chordal ones at runtime — meaning the Zygote/ ForwardDiff testsets in test_gaussian_approximation.jl ended up running the ChordalGMRF pipeline. Rename to `chordal_backends` and `test_gauss_approx_pipeline_chordal`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Turing 0.40 pins DynamicPPL ~0.37, which is incompatible with the Mooncake 0.5.25+ this PR requires. Allowing 0.41-0.44 lets the resolver land on Turing 0.44.4 + DynamicPPL 0.41.6, which compose with the new stack. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR-readiness fixes for ChordalGMRF
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I added a new type
ChordalGMRF <: AbstractGMRF, along with files for testing and benchmarking it.Performance depends on the merging of several PRs:
ProjectTofor symmetric sparse matrices. JuliaDiff/ChainRulesCore.jl#707accummethod for symmetric sparse matrices. FluxML/Zygote.jl#1609In the meantime, I added a
piracy.jlfile that implements the changes locally. Some type piracy also occurs in the Zygote.jl extension.Make sure to run this with CliqueTrees.
Benchmarks
Here is the output of
gaussian_approximation_comparison.jl.And here is the output of
logpdf_comparison.jl: